Skip to content

Suppress diff exceptions#167

Merged
AlexandrSuhinin merged 6 commits into
mainfrom
suhinin/suppres-diff-exceptions
Jun 5, 2026
Merged

Suppress diff exceptions#167
AlexandrSuhinin merged 6 commits into
mainfrom
suhinin/suppres-diff-exceptions

Conversation

@AlexandrSuhinin
Copy link
Copy Markdown
Collaborator

No description provided.

@AlexandrSuhinin AlexandrSuhinin requested a review from ishulgin June 1, 2026 11:41
@AlexandrSuhinin AlexandrSuhinin changed the title Suppres diff exceptions Suppress diff exceptions Jun 1, 2026
Comment thread src/CodexToolCallMapper.ts
Comment thread src/CodexToolCallMapper.ts Outdated
Comment thread src/CodexToolCallMapper.ts Outdated
type: "diff",
oldText: oldContent,
newText: newContent,
path: change.path,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if file was actually moved? Don't we want to use PatchChangeKind#moved_path?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think that it's impossible to express move in a single diff event. We probably need to split it into delete + add pair.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Confirmed
image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss how to properly send moved files. For now we send moved_path + oldText/newText.
Let's fixup this in following PR.

Comment thread src/CodexToolCallMapper.ts Outdated
function isUnifiedDiff(content: string): boolean {
return content.startsWith("--- ") || content.includes("\n--- ");
async function createUpdateFileContent(change: FileUpdateChange): Promise<ToolCallContent | null> {
if (change.kind.type !== "update") return null;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have similar guards in other execution branches. Let's drop it?

@AlexandrSuhinin AlexandrSuhinin merged commit 6ec28b8 into main Jun 5, 2026
4 checks passed
@AlexandrSuhinin AlexandrSuhinin deleted the suhinin/suppres-diff-exceptions branch June 5, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants